There are some annotation examples in emmylua_ls's files that might help, for example:
..\AppData\Local\emmylua_ls\resources\std\table\new.lua

If you're getting Claude (or w/e) to help, make sure it's only considering the latest (rust) version of EmmyLua, not the old .NET or Java ones.

The latest PR changes the way classes are constructed. Instead of the constructor being in the varargs of newClass, it's now a separately defined method, which is called by new().

Basically, class creation is now:

---@class Example
local ExampleClass = newClass("Example", "Parent1", "Parent2")
---@param whatever: WhatTypeItIs it's a good idea to type these
function ExampleClass:Example(whatever)
    self.whatever = whatever
end

Which means the class type has information that self.whatever exists. In most cases this isn't super useful as everything is any, but at least now typing these isn't completely useless. It also works well for discovering fields and methods, since now you can use the IDE "go to definition" button to go to, e.g. a method added by the parent class.

It's also possible to add more fields to the class with ---@field under it, which is a good idea if a field is only supposed to be initialised outside the class file.

At least with Sumneko's LSP, there were also warnings about making a class with a typo in the class name.

This linter PR also seems relevant, as these changes make linters much more useful: https://github.com/PathOfBuildingCommunity/PathOfBuilding-PoE2/pull/1796/changes

Using parent class constructors still looks a bit weird. For example self.Control() has a warning, because the LSP doesn't see that the function gets self supplied to it by new() via some sort of Lua magic. I'll look into how that can be fixed.

LoadModule() might also be possible with generics in a similar way. require for example already works properly with no annotations.

The last commit makes the proxy metatable (or whatever it's called) not supply self to the parent constructor. This means that you now use : to call it and type checking works correctly, as the LSP sees the self variable.

A local Codex-assisted analysis harness, followed by a Claude review of the findings, was used to sanity-check the current PR head (41d0976e1a353e20c2ea2ae741afedc8486a917e) against origin/dev.

Checks covered git diff --check, LuaJIT syntax loading for changed Lua files, normal headless startup, an Export startup smoke with SimpleGraphic stubs, focused luacheck on HeadlessWrapper.lua + _SimpleGraphic.def.lua, LuaLS diagnostics against origin/dev, and the Windows busted suite.

The pass found two runtime issues.

1. Export startup currently fails.

newClass now treats every vararg as a parent class name, but these Export classes still use the old trailing constructor function form:

src/Export/Classes/Dat64File.lua
src/Export/Classes/DatFile.lua
src/Export/Classes/DatListControl.lua
src/Export/Classes/GGPKData.lua
src/Export/Classes/GGPKSourceListControl.lua
src/Export/Classes/RowListControl.lua
src/Export/Classes/ScriptListControl.lua
src/Export/Classes/SpecColListControl.lua

The Export smoke fails with:

../Modules/Common.lua:72: attempt to concatenate local 'className' (a function value)

The minimal fix that passed locally was to convert those classes to the named constructor style already used by the migrated src/Classes/ files:

local XClass = newClass("X", "Parent")

function XClass:X(...)
    ...
end

2. _SimpleGraphic.def.lua is currently executed by HeadlessWrapper.lua, but its callback globals do not match the wrapper.

If _SimpleGraphic.def.lua is meant to stay definition-only, then HeadlessWrapper.lua probably should not dofile() it. If it is meant to provide the executable headless stubs, then SetCallback / GetCallback need to use the same callback table as the wrapper. The minimal executable-stub fix that passed locally was:

function SetCallback(name, func)
    __callbackTable__[name] = func
end

function GetCallback(name)
    return __callbackTable__[name]
end

In the current smoke this only matters if anything calls SetCallback or GetCallback after the headless wrapper runs; the symbol is undefined as written, so this looks like a latent bug rather than a guaranteed startup crash.

After applying those two local changes, these checks passed:

syntax check: pass
normal headless smoke: pass
export smoke: pass
luacheck smoke: pass
Windows busted suite: 224 successes / 0 failures / 0 errors / 0 pending

LuaLS also reports new diagnostics on existing dynamic patterns that the annotations currently model too narrowly, such as function-valued control shown / width / height, and Export data classes accessing fields without ---@field declarations.

Manual UI testing was not done.

Commit c0a223e changes new() to not call the constructor. This is motivated by the lack of type checking on varargs. It wraps the constructor to check parent initialisation. The construction syntax would now be different:

new("Control"):Control(nil, {0, 4, 0, 0})

The codebase was updated using a regex to do this. This is useful, because the LSP can now see what you should be putting into the constructor, instead of only seeing any-typed varargs:

But it does change the code style more than the previous changesm as those were strictly limited to class definition. Thoughts?

One old-style constructor call seems to remain after the latest constructor change.

src/Modules/ToastNotification.lua still creates the dismiss button with:

toast.dismissButton = new(
    "ButtonControl",
    ...
)

Since new() no longer forwards varargs to the constructor, that creates a ButtonControl object without running ButtonControl:ButtonControl(...). A local headless probe reproduces it by forcing a toast to render after its show animation:

Classes/Control.lua:80: attempt to index field 'anchor' (a nil value)

A scan of src + spec found this as the only remaining literal new("Class", ...) call with constructor args / without a :Constructor(...) chain.

The minimal conversion should be:

toast.dismissButton = new("ButtonControl"):ButtonControl(
    { "BOTTOMLEFT", anchorMain, "BOTTOMLEFT" },
    { 4, 0, 80, 20 },
    "Dismiss",
    function()
        dismissedIds[toast.id] = true
        toast.mode = "HIDING"
        toast.start = GetTime()
    end
)

So far the biggest remaining problem is the use of LoadModule() to import files. This works sort of similarly to require(), but it has some differences like being able to return multiple values and taking in arguments. require() would be a better option due to it having support for preserving the source file type info, but I think this PR is already large enough as is. It's probably better to do this later.

You can configure LoadModule to automatically have the capability of require.

  "runtime": {
    "version": "LuaJIT",
    "requireLikeFunction": ["LoadModule"]
  },

and write new like:

---@generic T
---@param classname `T`
---@return T
function new(classname)
end

get better lint and completion

You can configure LoadModule to automatically have the capability of require.

  "runtime": {
    "version": "LuaJIT",
    "requireLikeFunction": ["LoadModule"]
  },

That's true, but the behaviour of loadmodule is different. Some modules get an array passed to them which they then add variables to, for example. It would probably be better to just remove these cases, as they seem pretty weird. This does make sense for now, though.

Another pattern that I've noticed doesn't get type checked well is:

local x
if y then
    x = "string"
else
    x = 1
end

This seems to be quite common in the code base. Replacing these with functions might be required in some places.

Out of curiosity, since you're willing to fix the whitespace issues, why not choose to format the code? I see that the formatting in many places is still quite messy.

I'm not really involved with this project itself—just happened to stumble upon it, since you're using a tool I wrote.

Howdy! From what I gather It's "on the list" of things to do, but it's super low-priority. Also there is the concern that large format changes could potentially obfuscate bugs and make running things like git-blame trickier.

You can configure LoadModule to automatically have the capability of require.

  "runtime": {
    "version": "LuaJIT",
    "requireLikeFunction": ["LoadModule"]
  },

That's true, but the behaviour of loadmodule is different. Some modules get an array passed to them which they then add variables to, for example. It would probably be better to just remove these cases, as they seem pretty weird. This does make sense for now, though.

Commit ffed657 fixes this. Modules were converted to not use the non-standard behaviour of LoadModule() this means they can now be loaded with require(), which is what the other PR originally did. This was reverted, but it may still be a good idea to use require() in some places in the future, as it is cached unlike LoadModule().

PLoadModule() is still slightly problematic as it returns the error as the first value. If it was converted to return (mod, error) instead then it would be type checked properly. This is quite minor, though.

@Wires77 Would you be able to take a look at this? It seems like it'd be best to get this out of the way before there's more PR activity when patch notes and the league are out